tests/integration/defs/perf/test_perf_sanity.py (2)

1006-1012: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Avoid loading the whole log into memory to tail it.

readlines()[-num_lines:] materializes the entire file before slicing. A crashed trtllm-serve worker can emit very large logs, so this can spike memory (or OOM) exactly during CI triage. Iterating with a bounded deque keeps only the last num_lines in memory.

♻️ Bounded tail read

+from collections import deque
+
 ...
     try:
-        with open(log_path, "r", errors="replace") as log_file:
-            tail = "".join(log_file.readlines()[-num_lines:])
+        with open(log_path, "r", errors="replace") as log_file:
+            tail = "".join(deque(log_file, maxlen=num_lines))
     except OSError as err:

(Place the deque import with the other top-of-file imports.)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/defs/perf/test_perf_sanity.py` around lines 1006 - 1012,
The log tailing logic in the server log helper currently uses
readlines()[-num_lines:], which loads the entire file into memory before
slicing. Update the log-reading path to use a bounded deque for tailing instead,
and add the deque import alongside the other top-level imports. Keep the
existing OSError handling and print_error behavior in the same helper, but
ensure only the last num_lines lines are retained in memory.

---

1116-1131: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

wait() has no timeout and SIGKILL escalation never happens.

server_proc.terminate() sends only SIGTERM, then server_proc.wait() blocks unbounded. If the worker ignores SIGTERM (common for a hung server that is exactly what you're trying to triage), cleanup hangs indefinitely and the log tail is never printed. Note _server_exited_abnormally and its docstring already anticipate -SIGKILL, but nothing here escalates to kill().

This same block is duplicated in the ctx/gen branch (Lines 1344-1354) and the DISAGG_SERVER branch (Lines 1379-1389). Extracting a single helper resolves the hang and the duplication together.

♻️ Shared terminate-with-escalation helper

def _shutdown_and_maybe_echo(proc, log_path, label):
    raising = sys.exc_info()[0] is not None
    pre_rc = proc.poll()
    proc.terminate()
    try:
        rc = proc.wait(timeout=30)
    except subprocess.TimeoutExpired:
        proc.kill()
        rc = proc.wait()
    if raising or _server_exited_abnormally(pre_rc, rc):
        _echo_server_log_tail(log_path, label, pre_rc if pre_rc is not None else rc)

Then each finally reduces to a guarded call, e.g.:

         finally:
             if server_proc:
-                raising = sys.exc_info()[0] is not None
-                pre_rc = server_proc.poll()
-                server_proc.terminate()
-                rc = server_proc.wait()
-                if raising or _server_exited_abnormally(pre_rc, rc):
-                    _echo_server_log_tail(
-                        server_file_path,
-                        f"aggr:{server_idx}",
-                        pre_rc if pre_rc is not None else rc,
-                    )
+                _shutdown_and_maybe_echo(
+                    server_proc, server_file_path, f"aggr:{server_idx}"
+                )

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/integration/defs/perf/test_perf_sanity.py` around lines 1116 - 1131,
The shutdown path in the server cleanup block can hang indefinitely because
`server_proc.wait()` has no timeout and never escalates past SIGTERM. Update the
cleanup logic around `server_proc.terminate()` in this `finally` block, and in
the duplicated ctx/gen and `DISAGG_SERVER` branches, to use a shared helper (for
example, a `_shutdown_and_maybe_echo`-style routine) that waits with a timeout,
calls `server_proc.kill()` on timeout, then waits again before deciding whether
to call `_echo_server_log_tail`. Keep the existing `_server_exited_abnormally`
check and reuse the current `pre_rc`/`rc` handling so the log tail still prints
on failure.